Skip to content

improve SaR snapshot UI behavior#3595

Merged
georgweiss merged 1 commit into
ControlSystemStudio:masterfrom
High-Voltage-Engineering:dh-sar-snapshot-ui-behavior
Oct 22, 2025
Merged

improve SaR snapshot UI behavior#3595
georgweiss merged 1 commit into
ControlSystemStudio:masterfrom
High-Voltage-Engineering:dh-sar-snapshot-ui-behavior

Conversation

@DenSinH

@DenSinH DenSinH commented Oct 20, 2025

Copy link
Copy Markdown
Contributor

Basically, the SaR snapshot controller was a bit unintuitive. The UI suggested you needed to enter a name / description before hitting take snapshot / save, but hitting take snapshot would clear out the metadata entered by the user. I changed it so that the name and the comment are no longer cleared. However, if you start from an existing snapshot, this would preserve the name / description from that existing snapshot (which would be fine IMO, you can start editing from there) but the name needs to be unique. The error message shown to the user on a non-unique node name contained exception names because it was a nested exception. This is no longer the case.

Checklist

  • Testing: Tested new behavior locally

  • Documentation: I don't think this is significant enough to warrent a release not entry, nor is it breaking

@shroffk shroffk requested a review from georgweiss October 20, 2025 14:53
@shroffk

shroffk commented Oct 20, 2025

Copy link
Copy Markdown
Member

I changed it so that the name and the comment are no longer cleared

+1 thank you... such a simple but impactful change

@georgweiss georgweiss left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but found a bug when testing this.

#3597 fixes the bug and maybe this PR should pull that in.

@shroffk

shroffk commented Oct 21, 2025

Copy link
Copy Markdown
Member

@georgweiss I have merged the node setting PR... can I merge this and then we can test it again or do you th ink it is safer to update this branch with master and test again

@georgweiss georgweiss merged commit 8854527 into ControlSystemStudio:master Oct 22, 2025
2 checks passed
@georgweiss

Copy link
Copy Markdown
Collaborator

@shroffk, looks good, I merged it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants